Skip to content

RDKEMW-13122: Wi‑Fi networks in the picker are not ordered by signal strength#271

Open
gururaajar wants to merge 55 commits intodevelopfrom
topic/RDKEMW-13124
Open

RDKEMW-13122: Wi‑Fi networks in the picker are not ordered by signal strength#271
gururaajar wants to merge 55 commits intodevelopfrom
topic/RDKEMW-13124

Conversation

@gururaajar
Copy link
Contributor

Reason for change: Updated the onAvailableSSID to return strength and frequency as numbers so that the UI will do the sorting.
Test Procedure: Check the onAvailableSSID returns strength and frequency as numbers
Priority:P1
Risks: Medium
Signed-off-by: Gururaaja ESRGururaja_ErodeSriranganRamlingham@comcast.com

…strength

Reason for change: Updated the onAvailableSSID to return strength and frequency as numbers so that the UI will do the sorting.
Test Procedure: Check the onAvailableSSID returns strength and frequency as numbers
Priority:P1
Risks: Medium
Signed-off-by: Gururaaja ESR<Gururaja_ErodeSriranganRamlingham@comcast.com>
…strength

Reason for change: Updated the onAvailableSSID to return strength and frequency as numbers so that the UI will do the sorting.
Test Procedure: Check the onAvailableSSID returns strength and frequency as numbers
Priority:P1
Risks: Medium
Signed-off-by: Gururaaja ESR<Gururaja_ErodeSriranganRamlingham@comcast.com>
@gururaajar gururaajar requested a review from a team as a code owner February 3, 2026 16:14
Copilot AI review requested due to automatic review settings February 4, 2026 19:27
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 24 out of 24 changed files in this pull request and generated 5 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

double freq;
if (apFreq >= 2400 && apFreq < 5000)
freq = "2.4";
freq = 2.4f;
Copy link

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incorrect type suffix used. The variable is declared as double freq but the literal assigned here is 2.4f which is a float literal. This should either be 2.4 (double literal) or the variable should be declared as float.

Suggested change
freq = 2.4f;
freq = 2.4;

Copilot uses AI. Check for mistakes.
Comment on lines 1030 to 1087
@@ -953,36 +1060,36 @@ namespace WPEFramework
NMLOG_DEBUG("Received Noise (%d) from wifi driver is not valid; so clamping it", readNoise);
if (readNoise >= 0) {
readNoise = 0;
noise = std::to_string(0);
noise = 0;
}
else if (readNoise < DEFAULT_NOISE) {
readNoise = DEFAULT_NOISE;
noise = std::to_string(DEFAULT_NOISE);
noise = DEFAULT_NOISE;
}
}

/*Calculate SNR = RSSI - Noise */
int16_t calculatedSnr = readRssi - readNoise;
snr = std::to_string(calculatedSnr);
snr = calculatedSnr;

/* mapping rssi value when the SNR value is not proper */
if(!(calculatedSnr > 0 && calculatedSnr <= MAX_SNR_VALUE))
{
NMLOG_WARNING("calculated SNR (%d) is not valid; Lets map with RSSI (%s)", calculatedSnr, strength.c_str());
calculatedSnr = std::stoi(strength);
NMLOG_WARNING("calculated SNR (%d) is not valid; Lets map with RSSI (%d)", calculatedSnr, strength);
calculatedSnr = strength;
/* Take the absolute value */
calculatedSnr = (calculatedSnr < 0) ? -calculatedSnr : calculatedSnr;

snr = std::to_string(calculatedSnr);
snr = calculatedSnr;
}

NMLOG_INFO("SSID:%s, BSSID:%s, Band:%s, RSSI:%s, Noise:%s, SNR:%s", ssid.c_str(), bssid.c_str(), band.c_str(), strength.c_str(), noise.c_str(), snr.c_str());
NMLOG_INFO("bssid=%s,ssid=%s,rssi=%s,phyrate=%s,noise=%s,Band=%s", bssid.c_str(), ssid.c_str(), strength.c_str(), linkSpeed.c_str(), noise.c_str(), band.c_str());
NMLOG_INFO("SSID:%s, BSSID:%s, Band:%s, RSSI:%d, Noise:%d, SNR:%d", ssid.c_str(), bssid.c_str(), band.c_str(), strength, noise, snr);
NMLOG_INFO("bssid=%s,ssid=%s,rssi=%d,phyrate=%s,noise=%d,Band=%s", bssid.c_str(), ssid.c_str(), strength, linkSpeed.c_str(), noise, band.c_str());
Copy link

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inconsistent whitespace formatting. Lines 1030, 1086, 1087 use leading tabs while the surrounding code uses spaces for indentation. This should be consistent with the rest of the file's indentation style.

Copilot uses AI. Check for mistakes.
Comment on lines 1001 to 1013
strength = std::stoi(value);
rssiFound = true;
}
}
else if (key == "NOISE") {
noise = value;
if (!value.empty()) {
noise = std::stoi(value);
noiseFound = true;
}
}
else if (key == "AVG_RSSI") { // if RSSI is not available
if (strength.empty())
strength = value;
if (!rssiFound && !value.empty())
strength = std::stoi(value);
Copy link

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Potential exception safety issue. The code uses std::stoi() without any exception handling. If the value string contains non-numeric characters or values that cannot be converted to an integer, this will throw std::invalid_argument or std::out_of_range exceptions. Consider adding try-catch blocks around these conversions or validating the string format before conversion.

Copilot uses AI. Check for mistakes.
template <typename T>
T* baseInterface()
{
static_assert(std::is_base_of<T, Notification>(), "base type mismatch");
Copy link

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incorrect static_assert syntax. The second argument (error message) is missing from the static_assert call. The correct syntax should be static_assert(std::is_base_of&lt;T, Notification&gt;::value, "base type mismatch") to check the value member of the is_base_of trait, and provide an error message string as the second parameter.

Copilot uses AI. Check for mistakes.
Comment on lines 1011 to 1041
@@ -920,21 +1027,21 @@ namespace WPEFramework
band = "not known";
}
}
else if (key == "LINKSPEED")
else if (key == "LINKSPEED")
{
linkSpeed = value;
}
}
}
pclose(fp);

if (noise.empty())
noise = "0";
if (strength.empty())
strength = "0";
if (!noiseFound)
noise = 0;
if (!rssiFound)
strength = 0;
Copy link

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Logic issue with rssiFound flag. When RSSI is empty or not present, the code falls back to AVG_RSSI at line 1012-1013, but it doesn't set rssiFound=true. This means that at line 1040-1041, strength gets reset to 0 even if AVG_RSSI was successfully parsed. Either rssiFound should be set to true when AVG_RSSI is used, or the reset logic at line 1040-1041 should be removed since AVG_RSSI is a valid fallback.

Copilot uses AI. Check for mistakes.
Copilot AI review requested due to automatic review settings February 4, 2026 21:15
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 24 out of 24 changed files in this pull request and generated 3 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 56 to 69
MOCK_METHOD(uint32_t, RegisterInterfaceStateChangeNotification(IInterfaceStateChangeNotification* notification), (override));
MOCK_METHOD(uint32_t, UnregisterInterfaceStateChangeNotification(IInterfaceStateChangeNotification* notification), (override));
MOCK_METHOD(uint32_t, RegisterActiveInterfaceChangeNotification(IActiveInterfaceChangeNotification* notification), (override));
MOCK_METHOD(uint32_t, UnregisterActiveInterfaceChangeNotification(IActiveInterfaceChangeNotification* notification), (override));
MOCK_METHOD(uint32_t, RegisterIPAddressChangeNotification(IIPAddressChangeNotification* notification), (override);
MOCK_METHOD(uint32_t, UnregisterIPAddressChangeNotification(IIPAddressChangeNotification* notification), (override);
MOCK_METHOD(uint32_t, RegisterInternetStatusChangeNotification(IInternetStatusChangeNotification* notification), (override);
MOCK_METHOD(uint32_t, UnregisterInternetStatusChangeNotification(IInternetStatusChangeNotification* notification), (override);
MOCK_METHOD(uint32_t, RegisterAvailableSSIDsNotification(IAvailableSSIDsNotification* notification), (override);
MOCK_METHOD(uint32_t, UnregisterAvailableSSIDsNotification(IAvailableSSIDsNotification* notification), (override);
MOCK_METHOD(uint32_t, RegisterWiFiStateChangeNotification(IWiFiStateChangeNotification* notification), (override);
MOCK_METHOD(uint32_t, UnregisterWiFiStateChangeNotification(IWiFiStateChangeNotification* notification), (override);
MOCK_METHOD(uint32_t, RegisterWiFiSignalQualityChangeNotification(IWiFiSignalQualityChangeNotification* notification), (override);
MOCK_METHOD(uint32_t, UnregisterWiFiSignalQualityChangeNotification(IWiFiSignalQualityChangeNotification* notification), (override);
Copy link

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing semicolons at the end of MOCK_METHOD declarations. Each of these lines should end with a semicolon to properly close the macro invocation.

Suggested change
MOCK_METHOD(uint32_t, RegisterInterfaceStateChangeNotification(IInterfaceStateChangeNotification* notification), (override));
MOCK_METHOD(uint32_t, UnregisterInterfaceStateChangeNotification(IInterfaceStateChangeNotification* notification), (override));
MOCK_METHOD(uint32_t, RegisterActiveInterfaceChangeNotification(IActiveInterfaceChangeNotification* notification), (override));
MOCK_METHOD(uint32_t, UnregisterActiveInterfaceChangeNotification(IActiveInterfaceChangeNotification* notification), (override));
MOCK_METHOD(uint32_t, RegisterIPAddressChangeNotification(IIPAddressChangeNotification* notification), (override);
MOCK_METHOD(uint32_t, UnregisterIPAddressChangeNotification(IIPAddressChangeNotification* notification), (override);
MOCK_METHOD(uint32_t, RegisterInternetStatusChangeNotification(IInternetStatusChangeNotification* notification), (override);
MOCK_METHOD(uint32_t, UnregisterInternetStatusChangeNotification(IInternetStatusChangeNotification* notification), (override);
MOCK_METHOD(uint32_t, RegisterAvailableSSIDsNotification(IAvailableSSIDsNotification* notification), (override);
MOCK_METHOD(uint32_t, UnregisterAvailableSSIDsNotification(IAvailableSSIDsNotification* notification), (override);
MOCK_METHOD(uint32_t, RegisterWiFiStateChangeNotification(IWiFiStateChangeNotification* notification), (override);
MOCK_METHOD(uint32_t, UnregisterWiFiStateChangeNotification(IWiFiStateChangeNotification* notification), (override);
MOCK_METHOD(uint32_t, RegisterWiFiSignalQualityChangeNotification(IWiFiSignalQualityChangeNotification* notification), (override);
MOCK_METHOD(uint32_t, UnregisterWiFiSignalQualityChangeNotification(IWiFiSignalQualityChangeNotification* notification), (override);
MOCK_METHOD(uint32_t, RegisterInterfaceStateChangeNotification, (IInterfaceStateChangeNotification* notification), (override));
MOCK_METHOD(uint32_t, UnregisterInterfaceStateChangeNotification, (IInterfaceStateChangeNotification* notification), (override));
MOCK_METHOD(uint32_t, RegisterActiveInterfaceChangeNotification, (IActiveInterfaceChangeNotification* notification), (override));
MOCK_METHOD(uint32_t, UnregisterActiveInterfaceChangeNotification, (IActiveInterfaceChangeNotification* notification), (override));
MOCK_METHOD(uint32_t, RegisterIPAddressChangeNotification, (IIPAddressChangeNotification* notification), (override));
MOCK_METHOD(uint32_t, UnregisterIPAddressChangeNotification, (IIPAddressChangeNotification* notification), (override));
MOCK_METHOD(uint32_t, RegisterInternetStatusChangeNotification, (IInternetStatusChangeNotification* notification), (override));
MOCK_METHOD(uint32_t, UnregisterInternetStatusChangeNotification, (IInternetStatusChangeNotification* notification), (override));
MOCK_METHOD(uint32_t, RegisterAvailableSSIDsNotification, (IAvailableSSIDsNotification* notification), (override));
MOCK_METHOD(uint32_t, UnregisterAvailableSSIDsNotification, (IAvailableSSIDsNotification* notification), (override));
MOCK_METHOD(uint32_t, RegisterWiFiStateChangeNotification, (IWiFiStateChangeNotification* notification), (override));
MOCK_METHOD(uint32_t, UnregisterWiFiStateChangeNotification, (IWiFiStateChangeNotification* notification), (override));
MOCK_METHOD(uint32_t, RegisterWiFiSignalQualityChangeNotification, (IWiFiSignalQualityChangeNotification* notification), (override));
MOCK_METHOD(uint32_t, UnregisterWiFiSignalQualityChangeNotification, (IWiFiSignalQualityChangeNotification* notification), (override));

Copilot uses AI. Check for mistakes.
freqValue = value;
}
else if (key == "LINKSPEED")
else if (key == "LINKSPEED")
Copy link

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inconsistent whitespace: this line uses spaces for indentation while the surrounding code uses tabs. The indentation should be consistent with the rest of the file.

Copilot uses AI. Check for mistakes.
Comment on lines 140 to 343
@@ -252,7 +259,7 @@ namespace WPEFramework
virtual uint32_t StartWPS(const WiFiWPS& method /* @in */, const string& pin /* @in */) = 0;
virtual uint32_t StopWPS(void) = 0;
virtual uint32_t GetWifiState(WiFiState &state /* @out */) = 0;
virtual uint32_t GetWiFiSignalQuality(string& ssid /* @out */, string& strength /* @out */, string& noise /* @out */, string& snr /* @out */, WiFiSignalQuality& quality /* @out */) = 0;
virtual uint32_t GetWiFiSignalQuality(string& ssid /* @out */, int& strength /* @out */, int& noise /* @out */, int& snr /* @out */, WiFiSignalQuality& quality /* @out */) = 0;
virtual uint32_t GetSupportedSecurityModes(ISecurityModeIterator*& modes/* @out */) const = 0;

/* @brief Set the network manager plugin log level */
@@ -263,25 +270,76 @@ namespace WPEFramework
virtual uint32_t Configure(const string configLine/* @in */) = 0;

/* @event */
struct EXTERNAL INotification : virtual public Core::IUnknown
struct EXTERNAL IInterfaceStateChangeNotification : virtual public Core::IUnknown
{
enum { ID = ID_NETWORKMANAGER_NOTIFICATION };
enum { ID = ID_NETWORKMANAGER_INTERFACE_STATE_NOTIFICATION };

// Network Notifications that other processes can subscribe to
virtual void onInterfaceStateChange(const InterfaceState state /* @in */, const string interface /* @in */) = 0;
};

/* @event */
struct EXTERNAL IActiveInterfaceChangeNotification : virtual public Core::IUnknown
{
enum { ID = ID_NETWORKMANAGER_ACTIVE_INTERFACE_NOTIFICATION };

virtual void onActiveInterfaceChange(const string prevActiveInterface /* @in */, const string currentActiveInterface /* @in */) = 0;
};

/* @event */
struct EXTERNAL IIPAddressChangeNotification : virtual public Core::IUnknown
{
enum { ID = ID_NETWORKMANAGER_IP_ADDRESS_NOTIFICATION };

virtual void onIPAddressChange(const string interface /* @in */, const string ipversion /* @in */, const string ipaddress /* @in */, const IPStatus status /* @in */) = 0;
};

/* @event */
struct EXTERNAL IInternetStatusChangeNotification : virtual public Core::IUnknown
{
enum { ID = ID_NETWORKMANAGER_INTERNET_STATUS_NOTIFICATION };

virtual void onInternetStatusChange(const InternetStatus prevState /* @in */, const InternetStatus currState /* @in */, const string interface /* @in */) = 0;
};

/* @event */
struct EXTERNAL IAvailableSSIDsNotification : virtual public Core::IUnknown
{
enum { ID = ID_NETWORKMANAGER_AVAILABLE_SSIDS_NOTIFICATION };

// WiFi Notifications that other processes can subscribe to
virtual void onAvailableSSIDs(const string jsonOfScanResults /* @in */) = 0;
};

/* @event */
struct EXTERNAL IWiFiStateChangeNotification : virtual public Core::IUnknown
{
enum { ID = ID_NETWORKMANAGER_WIFI_STATE_NOTIFICATION };

virtual void onWiFiStateChange(const WiFiState state /* @in */) = 0;
virtual void onWiFiSignalQualityChange(const string ssid /* @in */, const string strength /* @in */, const string noise /* @in */, const string snr /* @in */, const WiFiSignalQuality quality /* @in */) = 0;
};

/* @event */
struct EXTERNAL IWiFiSignalQualityChangeNotification : virtual public Core::IUnknown
{
enum { ID = ID_NETWORKMANAGER_WIFI_SIGNAL_NOTIFICATION };

virtual void onWiFiSignalQualityChange(const string ssid /* @in */, const int strength /* @in */, const int noise /* @in */, const int snr /* @in */, const WiFiSignalQuality quality /* @in */) = 0;
};

// Allow other processes to register/unregister from our notifications
virtual uint32_t Register(INetworkManager::INotification* notification) = 0;
virtual uint32_t Unregister(INetworkManager::INotification* notification) = 0;
virtual uint32_t RegisterInterfaceStateChangeNotification(IInterfaceStateChangeNotification* notification) = 0;
virtual uint32_t UnregisterInterfaceStateChangeNotification(IInterfaceStateChangeNotification* notification) = 0;
virtual uint32_t RegisterActiveInterfaceChangeNotification(IActiveInterfaceChangeNotification* notification) = 0;
virtual uint32_t UnregisterActiveInterfaceChangeNotification(IActiveInterfaceChangeNotification* notification) = 0;
virtual uint32_t RegisterIPAddressChangeNotification(IIPAddressChangeNotification* notification) = 0;
virtual uint32_t UnregisterIPAddressChangeNotification(IIPAddressChangeNotification* notification) = 0;
virtual uint32_t RegisterInternetStatusChangeNotification(IInternetStatusChangeNotification* notification) = 0;
virtual uint32_t UnregisterInternetStatusChangeNotification(IInternetStatusChangeNotification* notification) = 0;
virtual uint32_t RegisterAvailableSSIDsNotification(IAvailableSSIDsNotification* notification) = 0;
virtual uint32_t UnregisterAvailableSSIDsNotification(IAvailableSSIDsNotification* notification) = 0;
virtual uint32_t RegisterWiFiStateChangeNotification(IWiFiStateChangeNotification* notification) = 0;
virtual uint32_t UnregisterWiFiStateChangeNotification(IWiFiStateChangeNotification* notification) = 0;
virtual uint32_t RegisterWiFiSignalQualityChangeNotification(IWiFiSignalQualityChangeNotification* notification) = 0;
virtual uint32_t UnregisterWiFiSignalQualityChangeNotification(IWiFiSignalQualityChangeNotification* notification) = 0;
};
Copy link

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a breaking API change that modifies the types of signal quality fields from strings to numeric types. This should be documented in the CHANGELOG.md file with a new version entry under the "Changed" category to inform API consumers about the breaking change.

Copilot uses AI. Check for mistakes.
Copilot AI review requested due to automatic review settings February 4, 2026 21:29
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 24 out of 24 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot AI review requested due to automatic review settings February 5, 2026 19:32
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 24 out of 24 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +181 to 194
double nmUtils::wifiFrequencyFromAp(guint32 apFreq)
{
std::string freq;
double freq;
if (apFreq >= 2400 && apFreq < 5000)
freq = "2.4";
freq = 2.4f;
else if (apFreq >= 5000 && apFreq < 6000)
freq = "5";
freq = 5;
else if (apFreq >= 6000)
freq = "6";
freq = 6;
else
freq = "Not available";
freq = 0;

return freq;
}
Copy link

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The frequency values returned by wifiFrequencyFromAp are band identifiers (2.4, 5, 6) rather than actual frequency values. This is inconsistent with how frequencies are handled elsewhere in the codebase. For example, in NetworkManagerGdbusUtils.cpp line 406, the frequency is calculated as ((double)freq/1000) which produces actual frequency values like 2.412 GHz or 5.180 GHz.

This inconsistency means that the onAvailableSSIDs event will report simplified band numbers (2.4, 5, 6) while GetConnectedSSID returns precise frequency values. Consider either:

  1. Removing wifiFrequencyFromAp and using the precise frequency calculation everywhere
  2. Documenting this intentional difference in behavior
  3. Making both APIs consistent in their frequency reporting

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant